Skip to content

fix: read the docs the same on a CRLF checkout - #990

Open
Dusk1e wants to merge 1 commit into
ProjectOpenSea:mainfrom
Dusk1e:fix/doc-scan-line-endings
Open

fix: read the docs the same on a CRLF checkout#990
Dusk1e wants to merge 1 commit into
ProjectOpenSea:mainfrom
Dusk1e:fix/doc-scan-line-endings

Conversation

@Dusk1e

@Dusk1e Dusk1e commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Problem

npm run test fails on main for anyone whose git checked out CRLF, which is the default on Windows:

1) public exports
     finds the docs it is supposed to read:
     named imports of @opensea/seaport-js in README code fences

The fence pattern is anchored on a newline:

const fence = /```[a-zA-Z]*\n([\s\S]*?)```/g

So it never matches ```js\r\n, and the README scan comes back empty. Converting README.md to LF and rerunning the same commit turns it green again, which is why CI has not seen this — it checks out LF.

The vacuity guard is what caught it, and it caught exactly what it was put there for. Without it the run would have been green while reading nothing: exports every symbol the README's code examples import and exports every helper a doc comment promises is exposed both filter a list and compare against [], so an empty input satisfies them. On a CRLF clone those two checks were passing without looking at anything.

Fix

Normalise line endings where the files are read, so every scan sees the same text whatever the checkout did.

importedSymbolsIn normalises its own argument too. That is not redundancy for its own sake — it makes the parse independent of how the caller obtained the text, which is what lets it be driven directly from a test.

Test

One case, feeding the same fence through both line endings and expecting the same symbol out of each. It does not read from disk, so it holds regardless of what the checkout did, and a CI run on LF catches a regression here as well.

On a CRLF checkout this suite goes from one failure to none; the full run is 244.

npm run test fails on main for anyone whose git checked out CRLF, which is
the default on Windows:

  1) public exports
       finds the docs it is supposed to read:
       named imports of @opensea/seaport-js in README code fences

The fence pattern is anchored on a newline:

  /```[a-zA-Z]*\n([\s\S]*?)```/g

so it never matches ```js\r\n, and the README scan comes back with nothing.
CI checks out LF and stays green, which is why this was not visible.

The vacuity guard is what caught it, and it caught what it was written for.
Without it the run would have been green with the two export checks reading
an empty document: both filter a list and compare against [], so an empty
input passes them.

Normalise line endings when the files are read, so every scan sees the same
text whatever the checkout did. importedSymbolsIn also normalises its own
argument, so the parse does not depend on how the caller got the text, and
that is what the new case drives -- the same fence through both line
endings. It is checkout independent, so a CI run on LF catches a regression
here too.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.04%. Comparing base (f618d19) to head (57b7d21).
⚠️ Report is 376 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #990      +/-   ##
==========================================
+ Coverage   98.27%   99.04%   +0.77%     
==========================================
  Files          35       56      +21     
  Lines       14526    20098    +5572     
  Branches      660     1117     +457     
==========================================
+ Hits        14276    19907    +5631     
+ Misses        245      183      -62     
- Partials        5        8       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants